Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed SD files not being imported completely #12

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Conversation

SamuelBehr
Copy link
Collaborator

Fixed SD files not being imported in their entire length if structures fail to be read by the reader (caused the import to stop); structures imported from SDF are now assigned the file name extended with the index of the structure in the file (formerly the count of successfull imports) if no name of the structure could be detected;

Please review!

…s fail to be read by the reader (caused the import to stop); structures imported from SDF are now assigned the file name extended with the index of the structure in the file (formerly the count of successfull imports) if no name of the structure could be detected;
@JonasSchaub
Copy link
Collaborator

Let me summarise the problem as I understand it: We did not configure the SDF reader to skip erroneous structures before. Which led to the whole import being aborted when an erroneous structure was encountered. Correct?
Question: Why not simply configure the reader to skip at the beginning (you can actually set this in the constructor)? Because we would not be able to log it then? Would the reader just implicitly without any message directly jump to the next parseable structure?

@FelixBaensch
Copy link
Owner

Let me summarise the problem as I understand it: We did not configure the SDF reader to skip erroneous structures before. Which led to the whole import being aborted when an erroneous structure was encountered. Correct? Question: Why not simply configure the reader to skip at the beginning (you can actually set this in the constructor)? Because we would not be able to log it then? Would the reader just implicitly without any message directly jump to the next parseable structure?

This is exactly the problem, if we set "skip" in the Ctor, we cannot log the problematic SDF entries. For me the code is okay.

@JonasSchaub
Copy link
Collaborator

And if there are multiple erroneous entries in a row in the input file? They will be skipped together, right?

@SamuelBehr
Copy link
Collaborator Author

And if there are multiple erroneous entries in a row in the input file? They will be skipped together, right?

Hmmm, yes, but it will lead to the counter of the structures getting faulty...

At this point, I might be forced to extend the IteratingSDFReader with a respective counter.

@JonasSchaub
Copy link
Collaborator

Hmmm, yes, but it will lead to the counter of the structures getting faulty...

Exactly my thinking. I would not put priority on this point, rather remove the statement that the counters/indices refer to the position in the input file from the doc.

But one possible solution would be to parse the SD file yourself, put everything between two lines of "$$$" in a String/buffer, and then apply the MOL reader to that. If simply extending the IteratingSDFReader does not work. I guess this class does nothing else than what I just described.

@FelixBaensch
Copy link
Owner

Hmmm, yes, but it will lead to the counter of the structures getting faulty...

Exactly my thinking. I would not put priority on this point, rather remove the statement that the counters/indices refer to the position in the input file from the doc.

But one possible solution would be to parse the SD file yourself, put everything between two lines of "$$$" in a String/buffer, and then apply the MOL reader to that. If simply extending the IteratingSDFReader does not work. I guess this class does nothing else than what I just described.

Please don't, it all misses the mark by a long way. Leave the indexation of names as it is and that's it. Anything else would be too much of a good thing.

@sonarcloud
Copy link

sonarcloud bot commented Aug 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@JonasSchaub JonasSchaub changed the base branch from main to production February 21, 2024 14:58
Copy link

sonarcloud bot commented Feb 21, 2024

Quality Gate Passed Quality Gate passed

Issues
7 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@JonasSchaub JonasSchaub merged commit 248ee59 into production Feb 21, 2024
2 checks passed
@JonasSchaub JonasSchaub deleted the SDF_Import_fix branch February 21, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants